Skip to content

Feature/sqlite compiletime#1180

Open
Donach wants to merge 6 commits into
wurstscript:masterfrom
Code-Fixxers:feature/sqlite-compiletime
Open

Feature/sqlite compiletime#1180
Donach wants to merge 6 commits into
wurstscript:masterfrom
Code-Fixxers:feature/sqlite-compiletime

Conversation

@Donach

@Donach Donach commented Apr 3, 2026

Copy link
Copy Markdown
Contributor

This PR adds compiletime support for sqlite operations. Not thoroughly tested, but simple stuff works:

Self-contained Examples:
SQLite.txt
SQLiteHelder.txt

@Frotty

Frotty commented Jun 6, 2026

Copy link
Copy Markdown
Member

Hi, as mentioned, will you do the standard library pendant? (provide api functions) else this isn't publicly usable unless users define @extern natives by themselves?

@Donach

Donach commented Jul 22, 2026

Copy link
Copy Markdown
Contributor Author

Updated this PR with the clean implementation (including handle validation, statement connection ownership tracking, auto-finalizing open statements when closing connections, and explicit interpreter error messages).

Also opened the WurstStdlib2 PR providing the native @extern bindings: wurstscript/WurstStdlib2#450

Reborn Reviewer added 4 commits July 22, 2026 13:16
- Fix Integer reference-equality bug in sqlite_close (== → .intValue())
  that silently leaked statements for connection handles > 127
- Fix sqlite_reset to call clearParameters() matching native SQLite
  sqlite3_reset semantics
- Add closeAllSqliteResources() lifecycle hook for interpreter shutdown
- Remove trailing blank line in CompiletimeTests.java
@Frotty

Frotty commented Jul 22, 2026

Copy link
Copy Markdown
Member

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 1742a4fbd7

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

try {
ResultSet rs = sqliteResultSets.get(statement.getVal());
if (rs == null) {
boolean hasResultSet = stmt.execute();

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Track non-query statements after stepping

When sqlite_step is called more than once on an INSERT/UPDATE/DDL statement before sqlite_reset, sqliteResultSets is still empty, so this path calls stmt.execute() again and repeats the write instead of returning the already-reached DONE state. This can duplicate compiletime mutations in helper code that probes/steps a statement again before reset/finalize; store per-statement completion state and only execute again after sqlite_reset.

Useful? React with 👍 / 👎.

* Closes all open SQLite resources (result sets, statements, connections).
* Called by the interpreter on shutdown to prevent resource leaks.
*/
public void closeAllSqliteResources() {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Close SQLite resources from the runner

This cleanup hook is never invoked by CompiletimeFunctionRunner or ILInterpreter (the provider is registered and then only used through NativesProvider.invoke), so if compiletime code opens a database and then throws or forgets sqlite_close, the long-lived compiler/LSP process keeps JDBC statements/connections open and can leave file-backed SQLite databases locked across subsequent builds. Please wire this into a finally/owned provider lifecycle instead of relying on an unreachable public method.

Useful? React with 👍 / 👎.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants